Skip to content

submission#18

Open
pgrach wants to merge 1 commit intoalexnaylor99:mainfrom
pgrach:Pavel_Grachev
Open

submission#18
pgrach wants to merge 1 commit intoalexnaylor99:mainfrom
pgrach:Pavel_Grachev

Conversation

@pgrach
Copy link

@pgrach pgrach commented Oct 13, 2023

sorry, it took me 2 hours to do this pull request... overestimated my ability

@pgrach
Copy link
Author

pgrach commented Oct 13, 2023

I am not sure i really understood how it works. Maybe next time it would be good to have a session dedicated specifically for this.

Copy link

@sashaTribe sashaTribe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are my reviews

load_dotenv()

# Initialize currency converter
currency_converter = CurrencyRates()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats cool, never thought to get a currency converter library. Saves lines of code 👍

currency_converter = CurrencyRates()

# Define the stocks to monitor and the email details
stocks_to_monitor = ["AAPL", "MSFT", "GOOGL", "TSLA", "NKE"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want your code to align with pep8 coding conventions, this variable name should all be in caps. This will help your peers understand that it is a constant and it will not be changed

receiver_email = os.environ.get('RECEIVER_EMAIL')

#Error Handling (in case user forgets setring-up .env)
try:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent you used try-catch blocks to catch errors, a good habit to take in

# Fetch the last 7 days of price data, calculate the average and prepare message
def check_seven_day_avg(stock, ticker):
hist = stock.history(period="7d")
avg = mean(hist["Close"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you added a statistics package to calculate the mean rather than writing functions for it

print(f"Error sending email: {e}")

# Schedule the process_tickers function to run every minute
schedule.every(1).minutes.do(process_tickers)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this only run as long as you run the code on the command line? What if you don't want to have your laptop on for this but keep this code running? What would you do?

previous_price_gbp = currency_converter.convert('USD', 'GBP', previous_prices[ticker])
diff = previous_price_gbp - current_price_gbp
if diff >= 0.01:
subject = f'Price Drop Alert for {ticker}'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very precise variable names

previous_prices[ticker] = stock.info['currentPrice'] # Fetching the current price in USD again.

# Send an email notification
def send_notification(subject, body):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should try these technologies instead of IFTTT, is it free? Do you need a subscription?

from statistics import mean

# Load environment variable from .env file
load_dotenv()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you need to do that? You should be more specific with your documentation


# Define the stocks to monitor and the email details
stocks_to_monitor = ["AAPL", "MSFT", "GOOGL", "TSLA", "NKE"]
previous_prices = {ticker: 0 for ticker in stocks_to_monitor}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that the initial dictionary? Will this be updated throughout the running of the code?

msg = f'Subject: {subject}\n\n{body}'
server.sendmail(email_user, receiver_email, msg)
except Exception as e:
print(f"Error sending email: {e}")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good error messages format, something that I have been lazy on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants